fix: StateEncodeParams API panic with Market Actor SectorContentChanged method
#411
Conversation
…tateEncodeParams panic fix: add CBOR marshalling for SectorContentChangedParams to prevent StateEncodeParams panic
chore: remove empty test-file
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a panic in the StateEncodeParams API when used with the Market Actor's SectorContentChanged method by addressing CBOR marshalling issues with slice type aliases. The root cause was that SectorContentChangedParams was defined as a type alias (= []SectorChanges) which doesn't automatically generate CBOR marshalling methods.
- Convert
SectorContentChangedParamsfrom type alias to proper type definition - Implement manual
MarshalCBORandUnmarshalCBORmethods following existing codebase patterns - Add required imports (
fmt,io) for CBOR operations
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| builtin/v17/miner/miner_types.go | Convert type alias to proper type, add CBOR methods and imports |
| builtin/v16/miner/miner_types.go | Convert type alias to proper type, add CBOR methods and imports |
| builtin/v15/miner/miner_types.go | Convert type alias to proper type, add CBOR methods and imports |
| builtin/v14/miner/miner_types.go | Convert type alias to proper type, add CBOR methods and imports |
| builtin/v13/miner/miner_types.go | Convert type alias to proper type, add CBOR methods and imports |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #411 +/- ##
==========================================
- Coverage 3.60% 3.60% -0.01%
==========================================
Files 733 733
Lines 199998 200268 +270
==========================================
Hits 7212 7212
- Misses 190519 190789 +270
Partials 2267 2267
🚀 New features to boost your workflow:
|
@rjan90 did you use cbor gen here? If not we should figure out how to get this done by cbor gen |
|
Did you try adding the type here ? Is there an issue with following that approach? |
|
should end up being a little similar to #378 |
|
Thanks for the pointers both! Will update the PR and get it closer/more similar to #378 |
Fixes StateEncodeParams API panic when called with Market Actor's SectorContentChanged method by adding SectorContentChangedParams to cbor-gen for proper CBOR marshalling across actor versions v13-v17. - Convert SectorContentChangedParams from type alias to proper type - Add SectorContentChangedParams to gen.go files for automatic generation - Regenerate CBOR marshalling methods using cbor-gen
|
Okay, pushed some changes to make it look closer/similar to 378, as well as another round of testing, and can confirm that I now do not get any panics: |
github-project-automation
bot
moved this from ⌨️ In Progress
to ✔️ Approved by reviewer
in FilOz
github-project-automation
bot
moved this from ✔️ Approved by reviewer
to 🎉 Done
in FilOz
Fixes: filecoin-project/lotus#13329
Root Cause
SectorContentChangedParams was defined as a type alias []SectorChanges, but CBOR-gen doesn't automatically generate marshalling methods for slice type aliases.
Solution